Skip to content

[FLINK-4573] Fix potential resource leak due to unclosed RandomAccess…#2556

Closed
lw-lin wants to merge 2 commits into
apache:masterfrom
lw-lin:raf-close
Closed

[FLINK-4573] Fix potential resource leak due to unclosed RandomAccess…#2556
lw-lin wants to merge 2 commits into
apache:masterfrom
lw-lin:raf-close

Conversation

@lw-lin
Copy link
Copy Markdown
Contributor

@lw-lin lw-lin commented Sep 27, 2016

…File in TaskManagerLogHandler

Thanks for contributing to Apache Flink. Before you open your pull request, please take the following check list into consideration.
If your changes take all of the items into account, feel free to open your pull request. For more information and/or questions please refer to the How To Contribute guide.
In addition to going through the list, please provide a meaningful description of your changes.

  • General
    • The pull request references the related JIRA issue ("[FLINK-XXX] Jira title text")
    • The pull request addresses only one issue
    • Each commit in the PR has a meaningful commit message (including the JIRA id)
  • Documentation
    • Documentation has been added for new functionality
    • Old documentation affected by the pull request has been updated
    • JavaDoc for public methods has been added
  • Tests & Build
    • Functionality added by the pull request is covered by tests
    • mvn clean verify has been executed successfully locally or a Travis build has passed

if (raf != null) {
raf.close();
}
return;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's just re-throw the exception to retain current behavior.

} catch (IOException ioe) {
display(ctx, request, "Displaying TaskManager log failed.");
LOG.error("Displaying TaskManager log failed.", ioe);
if (raf != null) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should never be false, as we would otherwise get an NPE and would never enter this block.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@zentol comments addressed; thanks!

@StephanEwen
Copy link
Copy Markdown
Contributor

I think this is good to merge...

@StephanEwen
Copy link
Copy Markdown
Contributor

Merging this...

StephanEwen pushed a commit to StephanEwen/flink that referenced this pull request Sep 29, 2016
…sed RandomAccessFile in TaskManagerLogHandler

This closes apache#2556
@asfgit asfgit closed this in 2afc092 Sep 30, 2016
@lw-lin lw-lin deleted the raf-close branch September 30, 2016 13:00
liuyuzhong pushed a commit to liuyuzhong/flink that referenced this pull request Dec 5, 2016
…sed RandomAccessFile in TaskManagerLogHandler

This closes apache#2556
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants